Skip to content

Add a extract-function-index pass - #3939

Merged
tlively merged 2 commits into
WebAssembly:mainfrom
tlively:extract-function-index
Jun 17, 2021
Merged

Add a extract-function-index pass#3939
tlively merged 2 commits into
WebAssembly:mainfrom
tlively:extract-function-index

Conversation

@tlively

@tlively tlively commented Jun 17, 2021

Copy link
Copy Markdown
Member

This is a useful alternative to extract-function when you don't know the
function's name.

Also moves the extract-function tests to be lit tests and re-uses them as
extract-function-index tests.

@tlively
tlively requested review from aheejin and kripken June 17, 2021 14:30
Comment thread src/passes/pass.cpp Outdated
"flatten", "flattens out code, removing nesting", createFlattenPass);
registerPass("extract-function-index",
"leaves just one function selected by index",
createExtractFunctionIndexPass),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
createExtractFunctionIndexPass),
createExtractFunctionIndexPass);

then the indentation below would be fixed by clang-format

;; CHECK-NEXT: )

(module
;; Use another function in the table, but the table is not used in the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
;; Use another function in the table, but the table is not used in the
;; Use a function in the table, but the table is not used in the

A downside of separate lit files is that we can't have a "conversation" about their connections...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm. Will whip something up to help out here.

;; CHECK-NEXT: )

(module
;; Use another function in the table, and the table *is* used. As a result,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
;; Use another function in the table, and the table *is* used. As a result,
;; Use a function in the table, and the table *is* used. As a result,

@tlively

tlively commented Jun 17, 2021

Copy link
Copy Markdown
Member Author

@kripken, I added a way to have multiple modules in a single lit test. PTAL!

Edit: If this looks good, I plan to split foreach.py into a separate commit that will go in first, but I'm keeping them together for now to demonstrate that foreach.py works as intended.

;; CHECK-NEXT: (func $foo
;; CHECK-NEXT: (call $bar)
;; CHECK-NEXT: )
;; CHECK-NEXT: )

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are manually-created, correct? I worry about us accumulating a lot of those and having a bad time some day when we need to update them. Adding foreach seems like it would increase the amount of such test code... How hard would it be to support auto updating in these eventually?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, these are manually created for now. I think it would be reasonable to update the auto update script in the near future to handle multiple multiple modules in a file and also to give it an option to emit checks on a per-module rather than a per-function basis.

tlively added 2 commits June 17, 2021 11:09
The major drawback of lit tests is that so far they have only supported a single
module per test file. This commit adds a new utility script that splits an input
file into multiple files and runs a command on each of them, giving lit tests a
simple way to test multiple modules per file.
This is a useful alternative to extract-function when you don't know the
function's name.

Also moves the extract-function tests to be lit tests and re-uses them as
extract-function-index tests.
@tlively
tlively force-pushed the extract-function-index branch from c0d2047 to d15fed6 Compare June 17, 2021 18:17
@tlively
tlively enabled auto-merge (rebase) June 17, 2021 18:18
@tlively
tlively merged commit ea0b9ee into WebAssembly:main Jun 17, 2021
@tlively
tlively deleted the extract-function-index branch June 17, 2021 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants